Add --base-path CLI option to override the URL path in the tilejson #1205
Conversation
|
I will refactor these functions/tests later. |
martin/src/utils/utilities.rs
Outdated
| if let Ok(uri) = path.parse::<Uri>() { | ||
| let mut result = uri.path(); | ||
| while result.len() > 1 { | ||
| result = result.trim_end_matches('/'); | ||
| } | ||
| return Ok(result.to_string()); | ||
| } | ||
| Err(BasePathError(path.to_string())) |
There was a problem hiding this comment.
wouldn't this be an infinite loop?
You don't need a loop - just do this:
| if let Ok(uri) = path.parse::<Uri>() { | |
| let mut result = uri.path(); | |
| while result.len() > 1 { | |
| result = result.trim_end_matches('/'); | |
| } | |
| return Ok(result.to_string()); | |
| } | |
| Err(BasePathError(path.to_string())) | |
| if let Ok(uri) = path.parse::<Uri>() { | |
| Ok(uri.path().trim_end_matches('/').to_string()) | |
| } else { | |
| Err(BasePathError(path.to_string())) | |
| } |
There was a problem hiding this comment.
Seems for "//" trim_end_matches would return empty string "".. That would fail our test
There was a problem hiding this comment.
but if you leave it, it will break format!("{base_path}/{}", path.source_ids) -- because it would create two slashes
sharkAndshark
force-pushed
the
basepath
branch
from
e9e36fc
to
83cd23a
Compare
martin/src/utils/utilities.rs
Outdated
| } | ||
| if let Ok(uri) = path.parse::<Uri>() { | ||
| let mut result = uri.path().to_string(); | ||
| while result.chars().last().is_some_and(|v| v == '/') && result.len() > 1 { |
There was a problem hiding this comment.
Use pop() back as the trim_end_matches would return "' for "//", any better way to use the trim methods?
There was a problem hiding this comment.
ok, lets keep it simple - use this method just for the user-supplied base_path parsing - in which case it should be empty if the user only passed /. And keep the original code for x-rewrite-url header.
I will look at it later, see if it can be optimized, but its not worth it now
There was a problem hiding this comment.
Actually It's not bad to use pop() here?
There was a problem hiding this comment.
But your suggestion is more simple
Try to fix #1185